Skip to content

Fix ajax no access error #23910

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
chippison wants to merge 26 commits into
base: 5.x-dev
Choose a base branch
from
Open

Fix ajax no access error #23910

chippison wants to merge 26 commits into from
+264 −124

Conversation

@chippison
Copy link
Contributor

@chippison chippison commented Dec 22, 2025

Description

Dev-19785

This PR is to fix the regression we had when we tried to redirect invalid session errors to login page (https://innocraft.atlassian.net/browse/UX-305).

We only want to force the redirect when we actually detect an invalid session issue and not to redirect ALL NoAccessExceptions. We will retain the original behaviour of NoAccessException for ajax calls to just fail silently and just returning the error message.

Testing:

NOTE: The only way I could test this fix was to try to do some datatable actions that will trigger saving of table preference. This would show errors and will refresh the page with no change in the sorting when the issue is not fixed. This happened when the regression was introduced and was noticeable on our Demo Site.
If the issue is fixed, the datatable should not give any errors and sorting should work (although it will not be saved because it is 'anonymous' user).

This should still error and redirect if a session becomes invalid (session expires or is deleted). I was able to test this by logging in as an admin to my local site. Then deleted my session (or changing the expire date to something in the past), then clicking on a new category/page or even just trying to sort the datatable.

Checklist

  • [✔] I have understood, reviewed, and tested all AI outputs before use
  • [✔] All AI instructions respect security, IP, and privacy rules

Review

@chippison chippison requested a review from a team December 23, 2025 03:46
@chippison chippison marked this pull request as ready for review December 23, 2025 03:46
@github-actions
Copy link
Contributor

github-actions bot commented Jan 7, 2026

If you don't want this PR to be closed automatically in 28 days then you need to assign the label 'Do not close'.

@github-actions github-actions bot added the Stale The label used by the Close Stale Issues action label Jan 7, 2026
@chippison chippison added the Do not close PRs with this label won't be marked as stale by the Close Stale Issues action label Jan 7, 2026
@sgiehl sgiehl removed Do not close PRs with this label won't be marked as stale by the Close Stale Issues action Stale The label used by the Close Stale Issues action labels Jan 15, 2026
@sgiehl sgiehl force-pushed the fix-ajax-no-access-error branch from b8ea408 to b795c1a Compare January 15, 2026 08:27
@sgiehl sgiehl added the Regression Indicates a feature used to work in a certain way but it no longer does even though it should. label Jan 15, 2026
@sgiehl sgiehl added this to the 5.7.0 milestone Jan 15, 2026
sgiehl

This comment was marked as outdated.

Sign in to view

@chippison
Copy link
Contributor Author

chippison commented Jan 16, 2026

Generally the changes might work, but are far away from "perfect". When working on such legacy parts it's important to also challenge the current implementation and consider fundamental changes instead of just relying on existing behavior.

I've used codex to create some code upon your changes, to implement a proper session timeout handling that is less error prone and improve the code overall a bit:

  • Relying on 401 status code might still cause problems, as plugins might send 401 status codes, that should not result in reloads. I've update that to use a custom header instead.
  • Setting a global Access state when the session has a real timeout makes the whole handling a lot more accurate, than trying to detect that based on the referrer.
  • As the more accurate timeout detection would prevent the session timeout message to be displayed after page reload this state is temporarily stored in a cookie
  • Properly differ between 401 (fully unauthorized requests) and 403 (authorized requests without permission)

See fix-ajax-no-access-error...fix-session-timeout-handling for the proposed changes.

Note: This was built upon my rough ideas. I haven't done a proper review or deeper testing yet

Thanks for your input on this @sgiehl.
It was really helpful to point me in the right direction.
I have merged your changes(fix-session-timeout-handling) to this PR.
I also added some code to make it work if anonymous users are allowed to view the site.
Hopefully my changes are all good. 😁

Cheers

@chippison chippison requested a review from sgiehl January 16, 2026 02:58
sgiehl

This comment was marked as outdated.

Sign in to view

chippison and others added 16 commits January 19, 2026 21:59
@chippison @sgiehl
Adding a session expired header to the NoAccessException error
eb57634
@chippison @sgiehl
put back old version of Access.php
799635f
@chippison @sgiehl
adding a flag to indicate session has expired
22ca0cb
@chippison @sgiehl
add indication that session is invalid/expired when token in request …
a7ff017 
…is not the same as token saved for user
@chippison @sgiehl
adding a check that login and token auth is null as indication that s…
d379f03 
…ession has expired or is invalid
@chippison @sgiehl
only enforcing 401 http code in header when session expired flag is t…
ba963d2 
…rue otherwise, just fail silently return error message
@chippison @sgiehl
Making sure we 'await' when saving testEnvironment as well as waiting…
5c08a49 
… for network to be idle before doing some page actions
@chippison @sgiehl
making widgetloader open a blank page first
3ec2253
@chippison @sgiehl
fixing test
e76a3e0
@chippison @sgiehl
adding options override
816b0bf
@chippison @sgiehl
remove commented out code
71c8b20
@chippison @sgiehl
changing the test so that it now just checks for the session expired …
a61f1c6 
…error message since we have put back most ajax calls to fail silently
@chippison @sgiehl
test using a different test fixture
f1a04ad
@chippison @sgiehl
adding back the test to check the error message is shown
121b0f1
@sgiehl
Improve detection of session timeouts
12a3eaa
@sgiehl
use custom header instead of status code
cfc87e6
@sgiehl sgiehl force-pushed the fix-ajax-no-access-error branch 3 times, most recently from 345e17b to ebc08c1 Compare January 20, 2026 09:04
@sgiehl sgiehl force-pushed the fix-ajax-no-access-error branch from ebc08c1 to 07714c8 Compare January 20, 2026 09:57
@sgiehl sgiehl requested a review from a team January 20, 2026 11:57
@sgiehl sgiehl dismissed their stale review January 20, 2026 11:57

outdated

@sgiehl sgiehl added the not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. label Jan 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sgiehl sgiehl sgiehl left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

not-in-changelog For issues or pull requests that should not be included in our release changelog on matomo.org. Regression Indicates a feature used to work in a certain way but it no longer does even though it should.

Milestone

5.7.0

Development

Successfully merging this pull request may close these issues.

3 participants

@chippison @sgiehl